Skip to content

LG-5931 Document the rest of analytics events #13#6288

Merged
stevegsa merged 3 commits intomainfrom
stevegsa-doc-analytics-events
May 3, 2022
Merged

LG-5931 Document the rest of analytics events #13#6288
stevegsa merged 3 commits intomainfrom
stevegsa-doc-analytics-events

Conversation

@stevegsa
Copy link
Copy Markdown
Contributor

@stevegsa stevegsa commented May 2, 2022

  • IDV_PERSONAL_KEY_DOWNLOADED (done previously)
  • IDV_PERSONAL_KEY_VISITED
  • IDV_PERSONAL_KEY_SUBMITTED
  • IDV_PHONE_CONFIRMATION_FORM
  • IDV_PHONE_CONFIRMATION_VENDOR

@stevegsa stevegsa force-pushed the stevegsa-doc-analytics-events branch from 2eed083 to f5e8762 Compare May 2, 2022 21:34
…bmitted, phone confirmation form, and vendor
Copy link
Copy Markdown
Contributor

@zachmargolis zachmargolis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Comment thread app/services/analytics_events.rb Outdated
@stevegsa stevegsa marked this pull request as ready for review May 2, 2022 23:05
@stevegsa stevegsa merged commit fd300af into main May 3, 2022
@stevegsa stevegsa deleted the stevegsa-doc-analytics-events branch May 3, 2022 02:55
Comment thread app/services/analytics.rb
IDV_INTRO_VISIT = 'IdV: intro visited'
IDV_JURISDICTION_VISIT = 'IdV: jurisdiction visited'
IDV_JURISDICTION_FORM = 'IdV: jurisdiction form submitted'
IDV_PHONE_CONFIRMATION_FORM = 'IdV: phone confirmation form'
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like this is still referenced on main here, causing a 500 for me locally:

Analytics::IDV_PHONE_CONFIRMATION_FORM => :verify_phone,

I hit it after entering my password for a newly created account. Surprised that's not covered in a feature spec 🤔

Copy link
Copy Markdown
Contributor

@aduth aduth May 4, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Surprised that's not covered in a feature spec 🤔

Oh, it is 😬

https://app.circleci.com/pipelines/github/18F/identity-idp/72761/workflows/e4ed12fd-723b-4d2c-8d4e-9d6144db792c/jobs/122933

But how did it pass here? 🤔 Edit: Ah, I think it was unfortunate merge timing with #6202, which added the reference to the constant.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants